Add OCI NoSQL Database: tables, rows, indexes

[arunesh-j]

Summary

• Implements OCI NoSQL Database Cloud Service against the existing portable database driver.
• Includes a real DDL parser, since OCI's CreateTable takes a SQL-ish statement rather than an attribute list.
• Nothing added to services/database/driver; OCI-only behaviour is a consumer-side Extras interface, per Move OCI-only capabilities out of shared driver packages #393.

Closes #412. Part of #376.

Changes

• providers/oci/nosql/ — Mock over memstore implementing driver.Database, guarded by a sync.RWMutex, plus the DDL parser and a query evaluator.
• server/oci/nosql/ — the /20190828/ surface.
• Wiring is one line each in providers/oci/oci.go and server/oci/oci.go.

Operations: tables (Create/List/Get/Update/Delete/ChangeCompartment), indexes (Create/List/Get/Delete), rows (GetRow/UpdateRow/DeleteRow), and Query. All 24 portable Database methods implemented.

DDL — what is parsed, what is refused

Parsed: CREATE TABLE and ALTER TABLE — scalar and JSON types, NOT NULL, DEFAULT, PRIMARY KEY with optional SHARD, USING TTL <n> DAYS, ADD/DROP on non-key columns, IF NOT EXISTS.

Refused by name, never silently accepted:

Refused	Why
Primary keys wider than two columns, composite shard keys	The portable partition/sort pair cannot identify such a row. This is a correctness rejection, not a shortcut
ARRAY / MAP / RECORD / ENUM	Not modelled
generated / MR_COUNTER / UUID modifiers	Not modelled
USING TTL … HOURS	OCI's own Schema reports TTL only in days
MODIFY, schema freezing, JSON-path index keys	Not modelled
Query projections, aggregates, joins, ORDER BY, range conditions	Not modelled

Judgement calls

• No change streams. OCI publishes no DynamoDB-Streams equivalent, so UpdateStreamConfig/GetStreamRecords return Unimplemented rather than an empty iterator. Noted in docs/services.md.
• Multi-delete is DELETE FROM … WHERE over /query — OCI's REST API has no MultiDelete operation; this is the real mechanism.
• ListIndexes requires compartmentId although real OCI marks it optional, so no list is ever unscoped. Documented.
• 501, named, for /tables/{id}/usage and /query/prepare, /query/summarize. usage is omitted from row and query responses for the same reason — zeros would read as real telemetry.

Provider Coverage

• AWS
• Azure
• GCP
• OCI

Checklist

• All tests pass (go test ./...) — exit 0, 272 packages
• Linter passes (golangci-lint run --timeout=9m) — 0 issues
• Every provider the change applies to implements the same behavior — OCI-only, additive
• Integration tests added to cloudemu_test.go — driver + handler tests instead
• Unit tests added to provider test files

Test Plan

go build ./...                                              clean
go test ./...                                               exit 0, 272 packages
go test -race ./providers/oci/... ./server/oci/...          11/11 ok
golangci-lint run --timeout=9m ./providers/oci/... ./server/oci/...   0 issues
go generate ./...                                           docs/coverage committed

Coverage leak check clean: no OCI operation in docs/coverage/{aws,azure,gcp}/*.md; git diff development -- services/ empty.

End-to-end on a running server (port 4613):

CreateTable                        -> 202 + Opc-Work-Request-Id
GetTable                           -> schema, primaryKey [id,email], shardKey [id], ttl 0
UpdateRow / GetRow                 -> 200 / value round-trips
ListTables                         -> 1 table; other compartment -> []; no compartmentId -> 400
CreateIndex                        -> 202 + work request; index ACTIVE
SELECT * FROM users                -> 3 rows
DELETE FROM users WHERE id = 2     -> NumRowsDeleted 2
DeleteRow twice                    -> 200, then isSuccess false
TRUNCATE TABLE                     -> 400 naming "TRUNCATE TABLE"
3-column PRIMARY KEY               -> 400 naming the limit
ARRAY(STRING)                      -> 400 naming "ARRAY"
USING TTL 6 HOURS                  -> 400 naming "HOURS"
ON_DEMAND + maxReadUnits           -> 400
undeclared column                  -> 400
SELECT name FROM users             -> 400 naming projections
GET /tables/users/usage            -> 501
DeleteTable                        -> 202; GetTable -> 404; ListTables -> []
USING TTL 7 DAYS                   -> schema.ttl 7; row carries timeOfExpiration

A note on parallel worktrees

An earlier run of this branch failed cmd/cloudemu TestServeOutOfProcess. It is not this change — cmd/cloudemu is untouched by the diff. Six Wave 2 worktrees were running their suites concurrently and that test contends on the shared ~/.cloudemu daemon lock. Verified: with nothing else running it passes, and the full suite is exit 0. Worth knowing as shared-state fragility whenever suites run in parallel.

[NitinKumar004]

Review notes

Real data-plane engine (per #427): N/A.

Findings

Medium · real-engine — Table names are globally unique, not per-compartment
providers/oci/nosql/nosql.go:417
If a user creates table "orders" in compartment A and then "orders" in compartment B -> the second CreateTable returns 409 AlreadyExists, because table names are a global key rather than compartment-scoped; real OCI scopes NoSQL table names per compartment and both creates succeed, so a multi-compartment SDK/CLI test that reuses a table name across compartments breaks against the emulator only.

m.tables is keyed by table name globally (nosql.go:175); newTable rejects with AlreadyExists on m.tables.Has(name) (nosql.go:417), and CreateOCITable checks m.tables.Get(d.Table) irrespective of spec.CompartmentID (table_extras.go:38-44). resolve()/lookup() also address tables by name globally.

Low · coverage — Portable Query/Scan operators and typed coercion below the 90% coverage pillar
providers/oci/nosql/rows.go:324
If a user issues a portable Query/Scan with a relational sort condition (>, BETWEEN) or stores a LONG/FLOAT/BOOLEAN-typed row -> the ordering (compareOrdering/compareStrings) and coercion (parseTyped/convertTyped) execute for the first time in production with no test guarding them, so a comparison- or coercion-logic regression would ship undetected.

go test -cover: provider 85.8%, server 83.4% (both < 90%). compareOrdering (rows.go:324) 0%, compareStrings (rows.go:341) 0%, compareOp (rows.go:306) 28.6%, parseTyped (row_extras.go:211) 35.7%, convertTyped (row_extras.go:242) 36.8%, SetMonitoring (nosql.go:194) 0%. Query tests exercise only SortOp "="; <, >, <=, >=, BETWEEN and CONTAINS/BEGINS_WITH ordering are never run.

Low · wire-fidelity — No oci-go-sdk SDK-compat test for the handler
server/oci/nosql/handler_test.go:94
If the real oci-go-sdk nosql client shapes a request/response field differently than the hand-modeled types.go structs (e.g. a body vs query-parameter for the Query limit, or a header the SDK requires) -> the divergence is not caught by the current tests, so a genuine client could fail at a step the httptest cases pass; adding one SDK create/get/query round-trip would close this.

handler_test.go drives the handler via raw httptest requests only; the only reference to github.com/oracle/oci-go-sdk is the package doc comment in handler.go. oci-conventions.md calls an SDK round-trip against httptest.NewServer "the strongest evidence the handler is right" (recommended, not required).

[NitinKumar004]

Review — OCI NoSQL Database

Verdict: Request changes — one blocker (persistence completeness), plus two local-gate failures and a rebase. The wire layer and lifecycle are strong; once these are addressed it's mergeable.

Blocking

Mock is not Snapshottable — providers/oci/nosql/nosql.go:168 (Mock).
The Mock holds tables and names memstore.Store fields, and each tableData nests an items row store, but there's no providers/oci/nosql/snapshot.go and no snapshot.Snapshottable. On a tree that includes the persistence-completeness guard:

--- FAIL: TestSnapshotCompleteness/oci
    oci: field NoSQL (*nosql.Mock) holds a memstore.Store but is not Snapshottable

Consequence: CI Test goes red, and a snapshot/restore of the OCI provider silently drops every table and all row data. Fix: add snapshot.go with var _ snapshot.Snapshottable = (*Mock)(nil) + Snapshot/Restore covering tables including each table's nested items rows (the easy-to-forget part), names, and per-table Scope/Tags/Indexes/ttl. Note tableData's unexported fields (items, ttl, Scope) must be promoted/handled to round-trip, mirroring providers/oci/vcn/snapshot.go.

Should fix

• 4× golangci-lint gocritic hugeParam — nosql.go:381 (CreateTable), rows.go:102 (UpdateItem), rows.go:206 (Scan), indexes.go:12 (CreateIndex). These are interface-fixed signatures; add the //nolint:gocritic // hugeParam: interface method signature cannot be changed. that providers/aws/dynamodb and this PR's own Query/CreateOCITable already carry. The golangci-lint run 0-issues bar (per CLAUDE.md) currently fails.
• Generated docs stale on rebase — docs/coverage/oci/nosql.md. go generate ./... on the merged tree drops an "Optional capabilities / TableAttributes" block the current coveragegen no longer emits; re-run go generate after rebase and commit.
• Branch is 781 commits behind development — rebase; only generated docs/coverage/README.md conflicts (code auto-merges).

Non-blocking

• Query numeric-literal formatting — query.go (rowMatches) / rows.go (compareOp). Comparisons stringify both sides via fmt.Sprintf("%v", …), so a DOUBLE 5.0 vs literal "5.0" (or a stored 5) can mismatch on formatting. Documented SQL subset; the exact-match integer path is verified. Minor fidelity edge.

Verified good

Full lifecycle over the wire (CreateTable DDL → 202 + work request resolving SUCCEEDED synchronously → GetTable ACTIVE with full schema → UpdateRow → GetRow round-trips id:1 as an integer, not 1.0 → Query → CreateIndex → delete → 404): computed OCID/timeCreated byte-stable; dup→409, missing→404, row-under-missing-table→404, bad DDL type→400, ON_DEMAND-with-units→400. Async work-request wiring correct and complete; error taxonomy faithful; rollback on CreateTable name/DDL mismatch; create paths span check-then-set under one write lock (no TOCTOU race); Get/List/Query/Index deep-copy (no aliasing; -race clean); honest 501 stubs for usage/query/prepare/summarize/change-streams; structure conformant.

[arunesh-j]

Rebased onto a45ac909 (781 commits) and addressed everything across 5da59199 and 95228653. Only docs/coverage/README.md conflicted, exactly as you predicted; code auto-merged and there was no services/database/driver drift.

Blocking — snapshot.go

Added, mirroring providers/oci/vcn/snapshot.go's shared storeDump table. go test ./persist/... now ok.

You were right that the nested items store is the easy-to-forget part — it needed custom MarshalJSON/UnmarshalJSON on tableData, carrying rows as a map and rebuilding the store on the way back. Renaming those two methods away makes TestSnapshotRestoreRoundTrip panic with invalid memory address or nil pointer dereference, which is exactly how the same shape failed on the Compute branch.

items and ttl travel through those methods; tableData is an unexported type, so there is no public API change. No lazily-minted defaults or counters to carry — Mock holds only tables, names, opts and monitoring, and OCIDs are minted at create and stored on the table.

One trap that was not on anyone's list: JSON decodes every number to float64, so an INTEGER key came back as float64(1) after restore. Added retypeRow, re-fitting rows to their declared column types. Reverting it fails with expected: int64(1) / actual: float64(1).

Round-trip tests cover tables including their rows, names, per-table Scope/Tags/Indexes/ttl, a TTL'd row still carrying its expiry, and malformed and empty input.

Per-compartment table names

The store is now keyed by compartment\x00name, following providers/aws/sagemaker's scopedKey. The OCID still resolves tenancy-wide via names, and every Extras method takes compartmentID alongside tableNameOrId — which mirrors OCI's own rule that a compartment is required when the path parameter is a table name and optional when it is an OCID. compartmentId was added to UpdateTableDetails, and fromCompartmentId to ChangeTableCompartmentDetails.

How the portable driver addresses a table, since #423 and #425 have the same finding and the answers should agree: it has no compartment in its shape, so it reads the compartment new resources default to, then the sole compartment holding that name — and leaves a name held by two others unaddressable rather than picking between them. ListTables returns exactly what that resolves. Cross-surface use keeps working, and ambiguity is never guessed.

Reverting tableKey to a bare name fails three tests with your exact AlreadyExists: table "users" already exists.

Also removed OCITableScope, which was declared on Extras and never called.

Engines (#427) — N/A confirmed, not assumed

config defines six engine hooks: Database, Cache, Function, Compute, Container, Storage. DatabaseEngine is relational only — it provisions a host and port for a client to connect with master credentials, is typed against services/relationaldb/driver, and dbengine dispatches on postgres/mysql/aurora-*/redshift. OCI NoSQL sits on services/database/driver (key-value/document); there is no NoSQL engine hook, and no sibling NoSQL provider (DynamoDB, Cosmos, Firestore) wires one.

Gate failures you named

• 4x gocritic hugeParam — fixed with the same targeted directive providers/aws/dynamodb and this PR's own Query/CreateOCITable already carry. golangci-lint is now 0 issues.
• Stale generated docs — go generate ./... re-run after the rebase and committed; running it a second time produces no diff.

Coverage

Provider 85.8% -> 91.0%, server 83.4% -> 95.3%. Every function you named is covered: all sort and filter operators on numeric and lexical keys, LONG/FLOAT/DOUBLE/NUMBER/BOOLEAN/BINARY/TIMESTAMP/JSON coercion including failures and DDL defaults, SetMonitoring via a capture stub, and the handler's work-request, malformed-body and error branches.

Numeric literals — fixed, not documented away

rowMatches is now type-aware: a condition on a numeric column compares by value, so DOUBLE 5 matches literal 5.0, while other columns stay textual and STRING "007" still does not equal 7. The portable compareOp cannot do this because it has no column types, so its equality limit is documented explicitly and points at rowMatches.

I chose fixing over documenting because a silent wrong answer is the accept-and-ignore shape this project has flagged repeatedly.

SDK-compat test — declined, with your questions answered

oci-go-sdk is a ~500-package monolith and vendoring it for one round-trip is disproportionate to a recommended-not-required item. I answered your two specific questions from the OCI spec instead: Query's limit is a body field on QueryDetails (matching queryRequest), and the handler sets opc-request-id, opc-work-request-id and opc-next-page — the SDK requires no header we do not set. Worth a follow-up issue rather than this PR.

Verification

go build ./...                                    clean
go test ./...                                     exit 0
go test ./persist/...                             ok        (the blocker)
go test -race (both nosql packages)               2/2 ok
golangci-lint (both nosql packages)               0 issues
go test -cover   provider 91.0%   server 95.3%
go generate ./...                                 committed and idempotent
git diff origin/development -- services/database/ empty (shared driver untouched)
no OCI op in docs/coverage/{aws,azure,gcp}/*.md   confirmed

E2E, port 4613

CreateTable                  -> 202 + Opc-Work-Request-Id
GetTable                     -> ACTIVE, primaryKey [id,sku], shardKey [id]
GetRow                       -> {"id":1,"qty":3,"sku":"widget"}   integer, not 1.0
Query                        -> 1 item
CreateIndex / GetIndex       -> 202, then ACTIVE
PRIMARY KEY (a,b,c)          -> 400 naming the 2-column limit
SHARD(a,b)                   -> 400 same
RECORD(x INTEGER)            -> 400 naming "RECORD"
GENERATED ALWAYS AS IDENTITY -> 400 naming the modifier
USING TTL 6 HOURS            -> 400 naming "HOURS"
ALTER ... MODIFY qty LONG    -> 400 naming the action
ALTER ... DROP id            -> 400, id is part of the primary key
same name in two compartments-> both created, distinct OCIDs, distinct capacity modes
row written to A             -> invisible from B (404)
delete B, then A             -> 202 each; A survives B's delete; both 404 after

On the cmd/cloudemu flake

Interim runs showed cmd/cloudemu persist/crash tests failing with shared-daemon collision markers while a sibling session ran its own suite. It is not this branch — the diff touches nothing under cmd/ or persist/ (verified: 0 files), and with nothing else running go test ./cmd/cloudemu/ passes on clean development in 5.4s. The cause is parallel worktree suites colliding on the shared ~/.cloudemu daemon lock.

[NitinKumar004]

The OCI NoSQL surface is in good shape overall, but one row-key collision loses data and a few wire behaviours differ from real OCI. These should be fixed before merge.

Earlier review points

• Mock not Snapshottable: fixed. snapshot.go round-trips tables with their nested rows, and TestSnapshotCompleteness and persist pass.
• Table names global instead of per-compartment: fixed with the compartment\x00name key.
• 4x gocritic hugeParam: fixed. golangci-lint reports 0 issues on the nosql packages and server/oci.
• Stale generated docs: fixed at the time. There is a new conflict now (see Low 1).
• Coverage below 90%: fixed. The provider is at 93.1% and the server at 95.6%.
• Numeric literal formatting in rowMatches: fixed.
• SDK-compat test: declined. One detail from that reply is wrong, see Low 2.

High

1. Composite string keys collide, so one row silently overwrites another. providers/oci/nosql/nosql.go:294 (itemKey)
   The store key is fmt.Sprintf("%v", shard) + ":" + fmt.Sprintf("%v", sort). That means (a="x:y", b="z") and (a="x", b="y:z") both map to x:y:z.
   Repro: CREATE TABLE kv (a STRING, b STRING, v STRING, PRIMARY KEY(SHARD(a), b)), then UpdateRow {"a":"x:y","b":"z","v":"first"} and UpdateRow {"a":"x","b":"y:z","v":"second"}. SELECT * FROM kv returns one row, and GetRow for a=x:y,b=z returns the second row. The portable driver uses the same key, so it is affected too.
   Fix: build the key so it can't collide. Either length-prefix each component or JSON-encode the key tuple, e.g. json.Marshal([]any{shard, sort}).

Medium

1. UpdateTable applies the ALTER even when the request fails. providers/oci/nosql/table_extras.go:143
applyAlter mutates the table in place, and the limits are validated afterwards.
   Repro: PUT /tables/users with ddlStatement: "ALTER TABLE users (ADD age INTEGER)" and tableLimits: {maxReadUnits:0, maxWriteUnits:0}. The call returns 400 InvalidParameter, but a following GetTable shows the age column and the rewritten ddlStatement. timeUpdated is not bumped either.
   Fix: validate the limits (and anything else that can fail) first, or apply the changes to a copy and swap it in only on success.

2. Row keys are percent-decoded twice. server/oci/nosql/row.go:120
r.URL.Query() already decodes the value, and decodeKey then runs url.QueryUnescape on it again. The SDK encodes key once (collectionFormat:"multi").
   Repro: store k="a+b". Then GET /tables/p/rows?key=k:a%2Bb returns 404, because the + becomes a space. Store k="50%". Then key=k:50%25 returns 400 "not valid percent-encoding". Those rows can't be read or deleted through GetRow or DeleteRow.
   Fix: drop the second QueryUnescape.

3. The shard key defaults to the leading column instead of the whole primary key. providers/oci/nosql/ddl.go:262
   In Oracle NoSQL, when no SHARD(...) is given, the shard key is the full primary key.
   Repro: CREATE TABLE users (id INTEGER, email STRING, name STRING, PRIMARY KEY(id, email)). GetTable then returns "shardKey":["id"], while real OCI returns ["id","email"]. The stored ddlStatement is also rewritten after an ALTER to PRIMARY KEY (SHARD(id), email), which is a different schema.
   Fix: report shardKey as the full primary key when no SHARD is declared. If the portable partition/sort mapping can't express that, keep that mapping internal and don't put it on the wire schema. Also preserve the user's DDL text.

4. IF_ABSENT and IF_PRESENT failures return 409 where OCI returns 200. providers/oci/nosql/row_extras.go:64 and server/oci/nosql/row.go:75
   In OCI, a conditional put that fails its option is a normal 200 with version null and existingVersion/existingValue set. A successful put returns version. Here a failed condition is 409 IncorrectState, and a success returns {}.
   Repro: UpdateRow with option: "IF_ABSENT" on an existing row returns 409 {"code":"IncorrectState"}. SDK callers check resp.Version != nil, so they get a ServiceError instead.
   Fix: return 200 with an UpdateRowResult. Set version on success (an opaque string stored per row works) and leave it null on condition failure. Add existingValue when isGetReturnRow is true.

5. UpdateTable by OCID skips the compartment visibility check. server/oci/nosql/table.go:121
   getTable, deleteTable, the row paths and changeCompartment all go through findTable, which returns 404 when the OCID belongs to another compartment. updateTable calls UpdateOCITable directly.
   Repro: GET /tables/<ocid>?compartmentId=<other> returns 404, but PUT /tables/<ocid> with compartmentId:<other> returns 202 and the tags change.
   Fix: call findTable in updateTable as the other handlers do.

Low

1. Merge conflict with current development: docs/coverage/README.md (generated). The code auto-merges. On the merged tree, go run ./internal/coveragegen only re-adds the NoSQL link in the database row, and the providers/oci, server/oci, persist and coveragegen tests pass. Rebase and regenerate.
2. Query limit is read from the body. server/oci/nosql/types.go:129. In QueryRequest, limit and page are query parameters, and QueryDetails has no limit field. Paging still works because paginate reads ?limit=. Drop the body field and pass ocirest.Limit(r) through.
3. isIfExists on DeleteTable is ignored. server/oci/nosql/table.go:134. DELETE /tables/users?isIfExists=true on a missing table returns 404. Honour the flag.
4. Compartment existence is not checked. Since #1295, development wires a compartment checker for VCN in server/oci/oci.go (nonexistent compartment returns 404 NotAuthorizedOrNotFound). This PR's handler accepts CreateTable into ocid1.compartment.oc1..doesnotexist with 202. Wiring the same SetCompartmentChecker here keeps OCI services consistent.
5. Table names are case-sensitive. Oracle NoSQL identifiers are case-insensitive, but GET /tables/USERS after CREATE TABLE users returns 404. Fold the case in tableKey and keep the declared spelling for display.
6. Work request operationType values are outside the NoSQL enum. The SDK enum is CREATE_TABLE, UPDATE_TABLE, DELETE_TABLE and UPDATE_CONFIGURATION. CREATE_INDEX, DELETE_INDEX and CHANGE_TABLE_COMPARTMENT (server/oci/nosql/handler.go:66-68) don't exist. Report index and compartment changes as UPDATE_TABLE.
7. isPrepared: true is silently ignored and the statement runs as plain text. Return 501 as /query/prepare does, so nothing is accepted and then ignored.
8. Responses carry no etag, and if-match is ignored. GetTable, GetRow and UpdateRow return etag in OCI. This matches other OCI handlers on development, so it can go in a follow-up issue.

Checked

• Clean build of the pushed SHA 9522865.
• Trial merge with development: one generated-file conflict. Build, vet and tests pass on the merged tree for providers/oci/..., server/oci/..., persist and internal/coveragegen.
• -race is clean on both nosql packages and server/oci. golangci-lint and gofmt report 0 issues.
• Live serve over raw HTTP: create, get twice (byte-identical), list with paging (opc-next-page), index create and get, row put, get and delete, query, DELETE FROM, delete followed by 404, and the work request resolving SUCCEEDED.
• Wire field names compared against the oci-go-sdk nosql request and response models.
• Terraform was not run. The oracle/oci provider only targets HTTPS nosql.<region>.oraclecloud.com and has no endpoint override, so raw HTTP against serve was used instead.
• Authorization: OCI has no enforce-auth gate on development yet, so that check does not apply to this handler.
• Wiring: providers/oci/oci.go, server/oci/oci.go and from_provider.go. The Snapshottable compile assert is present.

[NitinKumar004]

Please merge the latest development into this branch (a merge is fine, no need to rebase). It is well behind now, and docs/coverage/README.md conflicts. After merging, run go generate ./... and commit the regenerated docs/coverage, then push. I'll re-review once it's up to date with the findings above addressed.