Skip to content

fix(csharp): canonicalize identifier case in SEA get_primary_keys (PECO-3045) [SEA]#471

Open
eric-wang-1990 wants to merge 1 commit into
mainfrom
fix/csharp/PECO-3045-sea-primary-keys-case
Open

fix(csharp): canonicalize identifier case in SEA get_primary_keys (PECO-3045) [SEA]#471
eric-wang-1990 wants to merge 1 commit into
mainfrom
fix/csharp/PECO-3045-sea-primary-keys-case

Conversation

@eric-wang-1990
Copy link
Copy Markdown
Collaborator

What's Changed

The SEA path of GetPrimaryKeysAsync used the user-provided input names
(_metadataCatalogName / _metadataSchemaName / _metadataTableName)
verbatim when building result rows. When callers pass non-canonical case
(e.g. catalog=\"MAIN\" for a server-lowercase catalog main) the
TABLE_CAT / TABLE_SCHEM / TABLE_NAME result columns echo back the
input case instead of the canonical lowercase form. The Thrift path
returns canonical names from the server, and JDBC's SDK path reads
catalogName / namespace / tableName from the SHOW KEYS response —
so SEA was the outlier.

This change makes SEA read those three columns from each SHOW KEYS
record batch and use them when populating result rows, with a defensive
fallback to a lowercased copy of the user input if the columns are
missing or null. It mirrors how FetchCrossReferenceAsync already
reads parentCatalogName / parentNamespace / parentTableName from
SHOW FOREIGN KEYS in the same file.

Why

PECO-3045 — D17: SEA echoes input case in get_primary_keys result
columns; Thrift returns canonical names. JDBC spec says results should
be canonical.

Red → Green proof

Before fix (SEA_GetPrimaryKeys_ReturnsCanonicalCase_InResultColumns):

[xUnit.net 00:00:01.43]     ...SEA_GetPrimaryKeys_ReturnsCanonicalCase_InResultColumns [FAIL]
  Error Message:
   Assert.Equal() Failure: Strings differ
           ↓ (pos 0)
Expected: \"main\"
Actual:   \"MAIN\"
           ↑ (pos 0)
Failed!  - Failed:     1, Passed:     0, Skipped:     0, Total:     1

After fix:

Passed!  - Failed:     0, Passed:     1, Skipped:     0, Total:     1, Duration: 1 s

Full SeaMetadataE2ETests class — 19/19 pass post-fix.

Files touched

  • csharp/src/StatementExecution/StatementExecutionStatement.cs — read canonical names from SHOW KEYS columns; lowercase fallback.
  • csharp/test/E2E/StatementExecution/SeaMetadataE2ETests.cs — added SEA_GetPrimaryKeys_ReturnsCanonicalCase_InResultColumns.

Manual verification

  • E2E test red → green against pecotesting warehouse
  • Full SeaMetadataE2ETests class passes (19/19)
  • dotnet build clean on netstandard2.0, net472, net8.0
  • pre-commit — environment could not install pip deps (no network from sandbox); pre-existing CI hooks will run on PR

…CO-3045) [SEA]

The SEA path of GetPrimaryKeysAsync used the user-provided input names
(_metadataCatalogName / _metadataSchemaName / _metadataTableName) verbatim
when building result rows. When callers pass non-canonical case (e.g.
catalog="MAIN" for a server-lowercase catalog "main") the result columns
echo back the input case instead of the canonical lowercase form. The
Thrift path returns canonical names from the server and JDBC reads the
catalogName/namespace/tableName columns from SHOW KEYS, so SEA was the
outlier.

Fix: prefer the catalogName / namespace / tableName columns returned by
SHOW KEYS for each row, falling back to a lowercased copy of the user
input only if those columns are missing or null. Mirrors how SHOW FOREIGN
KEYS already populates parent identifiers in the same file.

PECO-3045
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant