fix(teradata): probe the CREATE TABLE right with the real destination DDL

[paulkarayan]

What & why

Problem: A Teradata destination user who cannot create the table the job auto-creates still finds that out at upload time rather than at the connector check, because the check asks a narrower question than the upload does. The probe that shipped in 1.11.18 creates a one-INTEGER-column table, while create_destination() runs the connector's real DDL with CLOB, VECTOR32, JSON and a PRIMARY INDEX. A right one of those column types needs and CREATE TABLE alone does not carry is invisible to the check, so the credential passes and the job fails partway through. Separately, a user whose per-workflow table already exists and who has since lost CREATE TABLE is now refused by that check with no way to act on the message, even though the upload would have found the table and never created one.

Change: The probe runs create_destination()'s own statement under the throwaway name, unqualified, so the rights it asks for are the rights the upload needs. Teradata 3523 refuses alongside 3524, with a message that does not claim CREATE TABLE is the missing right. Blank Database and unset Table Name each add a sentence naming the field that settles the refusal, so the customer has something to do. A refusal the server already gave survives a failure tearing the session down, and each precheck outcome gets its own log line.

Blast radius: 3/5 -- the connector check runs the real destination DDL on the customer's Teradata; one connector, revert-safe.

Linked ticket

none

Follow-up to #815, which merged before this work landed. This branch closes Trevor's review items 2, 3, 4, 5, 6 (traceability), 7, 10, 11a and 11b from #815 (comment), and the code half of item 1. Items 9 and the body half of 1 were fixed by editing the merged body of #815. Item 8 (version collision) and item 11c (TableKind = 'T') are answered in that thread and not changed here; the reasoning is in #815 (comment).

Provenance, so the diff is not mis-read. The first commit here, 7e6f164a "probe Teradata CREATE with the real destination DDL", was written in a separate earlier session against the branch of #815 and was still unpushed when that PR merged. It is not new work and it is not mine; this branch carries it forward unchanged, cherry-picked onto main (the trees matched exactly, so it applied clean). The second commit is the new work: the Table Name hint, its tests, and the CHANGELOG restructure.

Client-facing follow-up: a customer configured a Teradata destination with a non-admin user and a blank Database field; it passed the connector check and the run auto-created its table in a database the user had not chosen.

Impact

• Customers: Teradata destination users whose table is auto-created are now refused for the rights the real CREATE needs, not just CREATE TABLE on the database: a VECTOR32 column refused UDTUSAGE on SYSUDTLIB comes back as 3523 and is caught here instead of failing the job. Refusal messages become actionable rather than dead ends. With the Database field blank the message says the database it names is only the session default, so a DBA does not grant rights on a database nobody chose. With no Table Name configured the message says the check had to create a table to test the right, and that setting Table Name makes the check look the table up instead. That is the escape hatch for the one case this check can refuse a credential the job would not have needed. Destinations with a configured, existing table are unchanged: they still get the 1.11.16 INSERT/DELETE probe.
• Internal (devs / ops / other teams): support gets one INFO line per precheck outcome (skipped, refused, created, inconclusive) instead of a single line that could not distinguish "probe passed" from "skipped, table exists", and create_destination() now names the database on the line that actually creates the table. The probe table is named in a log line before the CREATE runs, so a pod killed between CREATE and DROP leaves a traceable name. No other connector is touched; _write_denied_message is called with the same arguments 1.11.18 introduced.
• Wire contract / clients: the precheck UserError (422) messages gain trailing sentences (the blank-Database hint from 1.11.18, plus the new Table Name hint). Teradata 3523 is a new refusal code for the destination precheck: a credential that previously passed the CREATE probe and failed at upload now gets a 422 at check time. test_teradata.py:1329 pins the upload-path wording and is unchanged. Nothing outside the teradata connector changes; the shared _USER_FAULT_TERADATA_CODES map is not edited by this branch.
• Deployment target considerations: the same on every target, because this is library code in the connector rather than anything deployed on its own. It runs wherever a Teradata destination precheck runs, which is per job where the preflight gate is enabled and on every UI test-connection. Not verified on any target with a real Teradata (see Proof).

Risk / rollback

• The probe now runs the real DDL, which is a larger statement than the one-column stand-in. It still creates and drops one table and commits under the driver's autocommit, so the leak window is unchanged: two statements. A failed DROP still passes the check and names the leftover.
• 3523 refusing is the one genuinely wider behaviour here. It is bounded to the CREATE probe's own except, and 3523 means a refused right and nothing else, so it cannot turn a missing table into a permissions error.
• No sweep of leftover probe tables was added, deliberately: bounded wrong it can drop a concurrent precheck's in-flight probe under the same credential.
• Revert the PR to back it out; nothing persists, no migration.

How it was verified

• make test-unit: 1843 passed. The teradata module alone is 168, up from 166, the two new ones being the Table Name hint firing when the field is unset and staying off when it is set.
• make check (ruff check .): all checks passed. ruff format was deliberately not run: the file was already format-dirty at HEAD before any edit here (verified with git show HEAD:<file> | ruff format --check), 17 files repo-wide fail ruff format --check, and CI's lint job runs ruff check only. Reformatting would have been unrelated churn on a review diff.
• The integration test asserts the precheck leaves no unstructured_precheck_% table behind, with _ escaped in the LIKE so the prefix cannot match by accident. It is gated on TERADATA_* credentials and skipped without them, so it did not run here.
• NOT verified against a live Teradata. The 3523 classification comes from Teradata's Database Messages manual and the existing _USER_FAULT_TERADATA_CODES, not from a server.

Proof

Proof waived (environment) -- no live Teradata is reachable from this machine. There are no TERADATA_* credentials locally and no Vantage SQL listener answers here. Unblocked by a Teradata Vantage (a ClearScape trial works) with an admin who can create a user lacking CREATE TABLE in one database, and a second lacking UDTUSAGE on SYSUDTLIB; then run the CREATE-probe red/green for both 3524 and 3523 against main and this branch, and land the restricted-user integration test modelled on test_postgres_destination_precheck_refuses_a_credential_that_cannot_write (test_postgres.py:247). That test is owed and is not in this branch.

Unit-level repro-first record for the new behaviour in this branch (the Table Name hint):

Red -- test written before the fix, against this branch's first commit:

$ uv run --locked --no-sync pytest -q test/unit/connectors/sql/test_teradata.py -k table_name_field
E   assert 'No Table Name is configured' in "The destination credentials can connect to the
    database but do not have CREATE TABLE permission on database 'test_db'. Records would fail
    to write. Grant CREATE TABLE on that database to the user this connector authenticates as."
FAILED test_teradata_precheck_refusal_points_at_the_table_name_field_when_it_is_unset
1 failed, 167 deselected in 0.41s

Green -- after adding _unset_table_hint() and appending it to both refusal messages:

$ uv run --locked --no-sync pytest -q test/unit/connectors/sql/test_teradata.py
168 passed in 0.64s

$ make test-unit
1843 passed, 1 warning in 37.62s

$ make check
uv run --locked --no-sync ruff check .
All checks passed!

The second test in that pair (..._omits_the_table_name_hint_when_one_is_configured) passes both before and after by construction; it is there to pin that the hint stays off for a customer who already set the field.

Dependencies / merge order

none

Note for whoever merges: this takes 1.11.20, renumbered from 1.11.19 when #814 shipped that version on main. #816 is still at 1.11.17 while main is at 1.11.19, and is currently conflicting. scripts/version-sync.sh:125 only rejects a version equal to main's, so a resolution there that keeps 1.11.17 goes backwards and CI will not catch it. Whichever of these two lands second needs its version re-checked by hand, not just its conflict resolved.

---

_{Generated from Orca worktree pk-teradata-precheck-followup (branch pk/teradata-precheck-review-followup).}

🤖 Generated with Claude Code

[cubic-dev-ai]

All reported issues were addressed across 5 files

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[awalker4]

One design-level point first: the CREATE probe is the one check in the SQL precheck family that leaves state behind.

The probe creates and drops a real table. _run_write_probe gets its answer from a zero-row statement and rolls back, so nothing persists. The CREATE probe commits a real table under autocommit and needs a second statement to remove it. If the DROP fails, or the process is killed between the two, an unstructured_precheck_* table stays in the customer's database, which is why the probe name is now logged before the CREATE. Running the full DDL means the table left behind is now a full-shape destination table.

Run it in a transaction and roll back. DDL is transactional in Teradata, so wrapping the CREATE in an explicit transaction (BT, the CREATE, then ROLLBACK) still gets the server's verdict on every right the real DDL needs, 3523 included, and nothing commits. A dropped session aborts the transaction, so there's no window in which a table can leak. The DROP, its failure path and the log line before the CREATE could then all go. I haven't run this against a live Teradata. Two things to confirm: Teradata-mode transactions require DDL to be the last statement, and the probe has to come off autocommit while it runs.

[tabossert]

Approving. Nothing here blocks. This closes most of the #815 list: the probe runs the real DDL and 3523 refuses too (2), both refusal messages carry the blank-Database hint (3), create_destination() names its database (4), a refusal survives teardown (5), the probe name is logged before the CREATE (6), the outer-except paths have tests (7), and each outcome has its own line (11b; a couple of gaps in 8 below). Unit CI is green; the SQL integration job was skipped, so as the body says, nothing here has hit a live Teradata. Notes below, most important first; 1 is the one to handle before merge.

1. Version: main took 1.11.19 while this was open. The body's merge note expected a clash with #816, but it was #814 (confluence) that merged at 1.11.19, so this needs 1.11.20. Only CHANGELOG.md conflicts: __version__.py merges cleanly because both sides made the identical 1.11.18 to 1.11.19 edit, so the resolution can keep 1.11.19 with no conflict marker pointing at it. scripts/version-sync.sh:125 would catch that, but check-version isn't a required check on main and its green run here predates main's bump. So: __version__ = "1.11.20", rename this section to ## [1.11.20] above main's ## [1.11.19], and confirm check-version goes green on the new head.

2. The CREATE probe refuses on 3523 and 3524 but still passes 5315 and 5612. The new docstring (teradata.py:642) says 3524 and 3523 are "the two codes that mean a right was refused and nothing else". The comment on _WRITE_DENIAL_TERADATA_CODES (:536-541) says the same of all four, {3523, 3524, 5315, 5612}. test_teradata.py:2389 pins 5315 as a pass because it's "not the CREATE TABLE answer", and that reasoning stopped holding once 3523 started refusing: by the PR's own comment at :551-552, 3523 isn't the CREATE TABLE answer either. The argument for adding 3523 (:543-546: the probe is the real DDL, so a right the server refuses it is one the real CREATE needs, and each code in the set "cannot mean anything else") covers 5315 and 5612 too. Either refuse on code in _WRITE_DENIAL_TERADATA_CODES with a message per code (and take 5315 out of the pass list at test_teradata.py:2389), or say in the code why those two still pass. I haven't checked whether a CREATE TABLE can return 5315 or 5612 at all; if it can't, a one-line comment saying so is enough.

3. Q: the 3523 message goes one step further than I asked for. In #815 I asked for a 3523 message that doesn't claim the missing right is CREATE TABLE. This one says it isn't: "The refused right is not CREATE TABLE on the database, which this code does not report" (teradata.py:771-772). The fixtures now disagree on that. The pre-existing create_destination() tests model a missing CREATE TABLE as 3523: _FakeTeradataDriverError("[Error 3523] User test_user does not have CREATE TABLE privilege") (test_teradata.py:1545-1547), and the redaction test at :1792 also uses a 3523 "no CREATE privilege" mock. The new probe tests (:2204, :2234) model CREATE TABLE as 3524 and UDTUSAGE as 3523. Those can't both be right, and as the body says, neither has been checked against a server. If a missing CREATE TABLE can come back as 3523, the new message sends a DBA away from the actual fix. Until it's verified live, I'd drop the clause ruling out CREATE TABLE and word the 3523 message as something like "a right on an object the table definition uses (for example UDTUSAGE on SYSUDTLIB)".

The same question runs the other way. In #815 I called 3524 the code that's certainly about CREATE TABLE, and on the real DDL I'm less sure. If I have Teradata's message template right, 3524 names its own database, so if a right on SYSUDTLIB itself came back as 3524, the message would name CREATE TABLE on the session database instead.

4. Neither the customer message nor our log says which right a 3523 refused. The message tells the customer "Teradata names the right it refused in the same error in the database's own log" (:774-775), but as far as I know that's only true if a DBA has query logging (DBQL, BEGIN QUERY LOGGING) covering this user or access logging (BEGIN LOGGING) turned on, and neither is guaranteed. Our own log doesn't have it either: _probe_error_detail (:831-833) emits the exception type plus teradata_error=3523. The right is in the driver text we deliberately drop. A narrow extraction would be safe: match does not have ([A-Z][A-Z ]{0,40}?) access to (Teradata's 3523 template is %FSTR does not have %VSTR access to %DBID.%TVMID, and 40 leaves room for EXECUTE FUNCTION WITH GRANT OPTION), only emit the capture if it's in a fixed list of Teradata right names, and fall back to today's text when nothing matches. It then goes in both the log line and the message, and host/user/password can't get through an allowlist.

5. The hints on the 3523 path aren't tested. The only test that drives the CREATE probe into 3523 (test_teradata.py:2221) uses the teradata_uploader fixture, which sets both database and table_name, so both hints are empty there. Deleting + self._blank_database_hint() + self._unset_table_hint() at :776 would leave the suite green: no test reaches that line with either hint non-empty. That test also doesn't assert the UDTUSAGE/SYSUDTLIB guidance. Parametrizing the hint assertions in :2251 and :2543 over 3523 and 3524 would cover it (:2280's CREATE TABLE wording assertion is 3524-only, so it stays on that case).

6. The Table Name hint doesn't say what to set it to. create_destination() uses self.upload_config.table_name or destination_name (:595), so a configured Table Name replaces the per-workflow name the platform passes; it doesn't just enable a lookup. The hint says "set the Table Name field" but not to what. Set to the existing table's exact name, writes stay where they were; set to anything else, the job writes to a different table. "Exact" matters: format_destination_name() sanitizes only the passed name (:594), never a configured Table Name. Can the customer even see the generated name? It's only logged when the table is created (:607). And if one connector can back several workflows (that's platform-side, so I haven't confirmed it), setting it would point all of them at one table. Worth saying "set Table Name to the existing table's exact name" at minimum, or checking whether the platform can pass the resolved name into precheck (the field is already marked x-runtime-eligible at :567, though what the platform does with that marker isn't in this repo).

7. Q: can the DROP fail on every check? Each call gets a fresh uuid4().hex[:16] name (:716), and a failed DROP warns and passes (:737-743), as the body says ("A failed DROP still passes the check and names the leftover") and test_teradata.py:2425 pins with a 3523 on the DROP. For a one-off failure that's fine. The case I'd want ruled out is a DROP refused on rights every time, which would leave another full-shape CLOB/JSON/VECTOR32 table per precheck with Table Name unset (or set to a table that doesn't exist yet). If Teradata grants the creator DROP TABLE on its own table automatically (I believe it does), that should be rare, so this is a question rather than a request. Refusing on a DROP failure wouldn't fit the probe's "no wider, no narrower" rule (:710-711) anyway, since the upload never drops anything; logging it at error would at least make a repeat visible. @awalker4's rollback suggestion would remove the path either way.

8. "One line per outcome" has a couple of gaps, and most of the new lines aren't tested.

• A teardown error after a successful CREATE and DROP (denial is None) logs "inconclusive" (:673) and returns, so the "can create" line (:684) never appears even though the probe was conclusive. This is the mirror of cubic's :664 note.
• A teardown error after the table-exists early return logs both the skip line and "inconclusive".
• The body says "one INFO line per precheck outcome", but a refusal logs at error (:727).
• The probe-name line, the "resolve to" line and the DROP warning have caplog checks (test_teradata.py:2331, :2381, :2437). I found none for "can create the destination table", "skipping the", "cannot create the destination table", "inconclusive", "refusal stands" or the new create_destination line (:607).

9. The leftover assertion I asked for in #815 can't run in CI, and passes whenever the probe creates nothing. sql_connectors_int_test runs only on schedule/dispatch/push (.github/workflows/e2e.yml:136-138), and no workflow sets any TERADATA_*, so @requires_env skips it there too. The module docstring already says it isn't wired into CI (test/integration/connectors/sql/test_teradata.py:16-21), which means "did not run here" also covers every automated run. Separately, assert leftover_probe_tables() == [] (:379) also passes when the CREATE comes back inconclusive and nothing gets created. And since it checks global state, one earlier leak makes it fail on every later run. Snapshotting before and after the precheck, plus a caplog check that the "probe creating" line (teradata.py:719-720) fired and no "inconclusive" did, would pin what it means to. The "can create" line (:684) isn't enough on its own, because it also fires after an inconclusive CREATE.

10. Nits.

• _run_write_probe wraps its classifier call in try/except (sql.py:540-552), so a classifier that raises is named at debug and the probe still falls through to inconclusive reporting the driver's error. _probe_table_creation claims "the same contract" (:707-708) but calls _classify_create_denial unguarded (:725); if it ever raised, the outer except would log "inconclusive" with the classifier's exception instead. Low odds.
• CHANGELOG (CHANGELOG.md:9) says "denied is set before the try"; the variable is denial now (:647).
• The body calls the blank-Database hint "from 1.11.18", but it isn't in 1.11.18's code; it's new in this branch's first commit. The CHANGELOG has it right.

Q. What level does the platform run the unstructured_ingest logger at? The probe-name line that's "the only record" of a leaked table (:717-720) is info, so at WARNING a leak from a process killed mid-probe has no record.

Pre-existing, not this PR's to fix:

• FYI, no change needed: the local Pipeline calls init() before precheck() (pipeline/pipeline.py:141-142), so there create_destination() hits the CREATE first and precheck sees an existing table. The new messages only reach callers that run precheck() without init(), which per the comment at :917-919 is the platform.
• get_connection() sets no logon or request timeout (:340-348), so a conflicting lock on the database could stall test-connection on the CREATE. It's heavier DDL now; if teradatasql has a request-timeout parameter, bounding the probe might be worth it.

cubic's three notes and @awalker4's transaction/rollback point are already in the thread, so I didn't repeat them.

[tabossert]

Approving. Notes are in my comment above; none of them block.

[paulkarayan]

All three cubic findings were real. Fixed, one commit each.

teradata.py:664 -- an inconclusive probe was certifying the right. _probe_table_creation() returned None for two different outcomes: a CREATE the server accepted, and a CREATE that failed for a reason the check cannot read. The caller treated both as probed, so a probe that died on 2644, 3803, 5315 or any unrecognized code logged "CREATE TABLE permission check inconclusive" and then, two lines later, "destination credentials can create the destination table in database X". The second line reports a result nobody got. The probe now returns the refusal alongside whether the CREATE was accepted, and the certifying log fires only on the accepted branch. Passing is unchanged: an inconclusive probe still passes, it just no longer claims a right it did not observe. A DROP that fails still counts as created, because the CREATE the server took is what proves the right. Covered both ways: the existing ..._fails_for_another_reason case now asserts the certifying line is absent, and a new test asserts it is present when the CREATE succeeds.

test_teradata.py:243 -- the leftover-probe-table check could not fail. leftover_probe_tables() filtered DBC.TablesV on TERADATA_DATABASE. With that blank, which is a supported configuration and the one the blank-Database hint exists for, the connector's probe lands in the session default instead, the filter matches no rows, and assert leftover_probe_tables() == [] passes without having looked anywhere. The helper now resolves the database with SELECT DATABASE, the way the connector resolves it.

CHANGELOG.md:5 -- the note overclaimed. "No identifier reaches the SQL text" is wrong: the probe table name is interpolated into both the CREATE and the DROP, deliberately. What stays out is the resolved database name, which is the point being made, since the statement is unqualified. Reworded to say that.

None of this touches the property the change is for: the probe still runs create_destination()'s own DDL through _elements_schema_sql(), so the rights it asks for are still the rights the upload needs.

Rebased onto main separately from the fixes. One conflict, in CHANGELOG.md: #814 shipped 1.11.19 while this branch was open, so the release note here is renumbered to 1.11.20 and __version__.py moves with it. pyproject.toml takes its version dynamically and uv.lock records the project as editable, so the lock is unchanged (uv lock --check clean).

make test-unit: 1864 passed.