fix(none): refuse a Teradata destination that cannot create its table

[paulkarayan]

What & why

Problem: A non-admin Teradata user passes the destination connector check even when it cannot create the table the job is about to create, because the check never asks; the job asks instead, halfway through, and is told the server is unreachable. That happens whenever the configured table does not exist yet or no table is configured: create_destination() builds it at upload time, which needs CREATE TABLE, and on main the 3524 it gets back surfaces as "Failed to connect to server". With the Database field blank it is worse, because the table goes into whatever database the session defaults to, which can be one the user never meant to write to, and nothing tells anyone which database that was.

Change: The Teradata destination precheck now resolves the database the write lands in (SELECT DATABASE, which is the configured one when set, else the session default), logs it, and creates and drops a throwaway unstructured_precheck_<hex> table in that database unless the configured table already exists there. With no table configured the probe always runs, since the platform names the table per workflow only when it calls create_destination(). Teradata error 3524 refuses the destination with a UserError that names the database; anything else passes.

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

This completes #811, which made the SQL precheck probe INSERT and DELETE on an existing table and called out the missing-table case as the one gap it left.

Linked ticket

none

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 now find out at connector check time that the user lacks CREATE TABLE, with a message naming the database to grant it on, instead of a job that fails with "Failed to connect to server". Destinations whose configured table already exists see no change. Only 3524 refuses. Correction (review follow-up): an earlier version of this line said a destination that works today keeps passing. That is wrong, and the Risk section below has it right. With no table configured the probe runs on every precheck, so a user whose per-workflow table already exists but who has since lost CREATE TABLE is now refused even though the upload would have found that table and never created one. Precheck cannot know the per-workflow table name, so it cannot tell the two cases apart. This bends fix(sql): refuse a destination credential that cannot write #811's no-wider rule (sql.py:380-382) on that one path.
• Internal: support gets the resolved database in the uploader's INFO log (destination writes resolve to Teradata database '<db>'), which is the fact that was missing when triaging where an auto-created table went. 3524 at upload time is now classified as the user's to fix (422) rather than a connection failure.
• Wire contract / clients: a new UserError (422) from the Teradata destination precheck: "The destination credentials can connect to the database but do not have CREATE TABLE permission on database ''. ...". SQLUploader._write_denied_message gained optional object_kind / object_name arguments; its default output is unchanged for every other dialect. 3524 raised from create_destination() at upload time changes from DestinationConnectionError to UserError "Teradata error 3524 (user does not have the required access to the database) for ''." (the descriptor is the one in _USER_FAULT_TERADATA_CODES, with the target appended by _raise_classified_teradata_error; test_teradata.py:1329 pins that wording). The same map is shared by the source side, so TeradataIndexer.precheck's SELECT TOP 1 probe now raises UserError rather than SourceConnectionError on 3524 as well.
• Deployment target considerations: same on SaaS, DI, in-VPC and on-prem (including the Teradata OEM sites): the check talks only to the customer's own Teradata. Not exercised against a real Teradata on any target.

Risk / rollback

• The precheck is slower on Teradata. It opens one more session and runs up to four more statements (SELECT DATABASE, a DBC.TablesV lookup when a table is configured, and the CREATE / DROP when that table is absent or none is configured). The preflight controller treats a precheck TIMEOUT as PASS, so on a slow logon this check can now time out and pass where it used to finish. That is the fail-open direction: the job still fails as it does today, it does not refuse a working destination.
• The check writes DDL. The CREATE commits under the driver's autocommit. If the DROP fails, a unstructured_precheck_<hex> table is left in the destination database and a warning names it; the check still passes. Every connector check on a destination with no table configured creates and drops one probe table, including after the workflow's own table exists.
• Only 3524 refuses. A user refused with some other code (5315, perm space, a bad database name) passes the check and fails at upload as before.
• With no table configured, a user whose per-workflow table already exists but who has since lost CREATE TABLE is now refused, though the job would have written to the existing table. Precheck cannot know the per-workflow table name, so it asks for the right a first run needs.
• Revert the PR to back it out; nothing persists.

How it was verified

• New unit tests cover: table absent and CREATE refused with 3524 (refused, database named, driver text absent); table absent and CREATE allowed (probe created then dropped); table present (no CREATE or DROP; the INSERT/DELETE probe still runs); blank Database (SELECT DATABASE names the database in the probe); no table configured (the probe runs without a lookup); the server's spelling of the database is what gets quoted; other CREATE failures pass; a failed DROP passes and names the leftover. All of them fail against origin/main's module.
• The full unit suite passes (pytest -n auto test/unit --ignore test/unit/unstructured), including the other SQL dialects that share _write_denied_message.
• A fake-driver simulation drives the real TeradataUploader against a modelled session default database and per-database grants; output under Proof.
• NOT verified against a live Teradata. The 3524 code and the DBC.TablesV lookup come from Teradata's documentation and the existing create_destination(), not from a server.

Proof

Proof waived (environment) -- no live Teradata is reachable from this machine. There are no TERADATA_* credentials locally (the live integration tests in test/integration/connectors/sql/test_teradata.py expect them exported by hand, and CI does not set them), 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; then run the CREATE-probe red/green against main and this branch.

Repro (local, fake teradatasql driver, real TeradataUploader from origin/main). The "new precheck" rows are main's precheck, which already includes the INSERT/DELETE probe (host replaced with <host>):

B. blank Database, operator's default db IS admin_db, no rights there
  new precheck      connector check: PASS
                    job:              FAIL  DestinationConnectionError: Failed to connect to server <host>
                    table now in:     nowhere

E. Database explicitly set to admin_db, no rights there
  new precheck      connector check: PASS
                    job:              FAIL  DestinationConnectionError: Failed to connect to server <host>
                    table now in:     nowhere

Failing tests against origin/main's module:

FAILED test_teradata_uploader_precheck_with_table_name_none
FAILED test_teradata_precheck_refuses_a_credential_that_cannot_create_the_missing_table
FAILED test_teradata_precheck_creates_and_drops_a_probe_table_when_the_table_is_missing
FAILED test_teradata_precheck_issues_no_create_when_the_table_exists
FAILED test_teradata_precheck_probes_the_session_database_when_database_is_blank
FAILED test_teradata_precheck_passes_when_the_create_probe_fails_for_another_reason[2644]
FAILED test_teradata_precheck_passes_when_the_create_probe_fails_for_another_reason[3803]
FAILED test_teradata_precheck_passes_when_the_create_probe_fails_for_another_reason[5315]
FAILED test_teradata_precheck_passes_when_the_create_probe_fails_for_another_reason[9999]
FAILED test_teradata_precheck_passes_and_names_the_leftover_when_the_drop_fails
FAILED test_teradata_precheck_quotes_the_database_as_the_server_spells_it

After (same simulation, this branch). Rows A, C and D do not move; B and E now stop at the connector check (destination table name and host replaced with <table> and <host>):

A. blank Database, operator's default db is its own, no rights in admin_db
  new precheck      connector check: PASS
                    job:              PASS
                    table now in:     ['operator_db.<table>']

B. blank Database, operator's default db IS admin_db, no rights there
  new precheck      connector check: FAIL  UserError: The destination credentials can connect to the database but do not have CREATE TABLE permission on database 'admin_db'. Records would fail to write. Grant CREATE TABLE on that database to the user this connector authenticates as.

C. blank Database, default db IS admin_db, rights there (direct or via a role)
  new precheck      connector check: PASS
                    job:              PASS
                    table now in:     ['admin_db.<table>']

D. blank Database, default db is own, table of that name ALREADY in admin_db
  new precheck      connector check: PASS
                    job:              PASS
                    table now in:     ['admin_db.<table>', 'operator_db.<table>']

E. Database explicitly set to admin_db, no rights there
  new precheck      connector check: FAIL  UserError: The destination credentials can connect to the database but do not have CREATE TABLE permission on database 'admin_db'. Records would fail to write. Grant CREATE TABLE on that database to the user this connector authenticates as.

Row C is the case to keep in mind: a user who holds CREATE TABLE in the session's default database still passes and the table still lands there. The check now logs which database that is; it does not stop it.

Dependencies / merge order

none

--- 🤖 Generated with Claude Code

[cubic-dev-ai]

1 issue found across 5 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="unstructured_ingest/processes/connectors/sql/teradata.py">

<violation number="1" location="unstructured_ingest/processes/connectors/sql/teradata.py:653">
P2: Escape double quotes in the resolved database identifier before building the qualified probe SQL. As written, databases containing a quote make the probe fail and the outer check treats that failure as a pass, so the destination can still fail later during table creation.</violation>
</file>

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

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Escape double quotes in the resolved database identifier before building the qualified probe SQL. As written, databases containing a quote make the probe fail and the outer check treats that failure as a pass, so the destination can still fail later during table creation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At unstructured_ingest/processes/connectors/sql/teradata.py, line 653:

<comment>Escape double quotes in the resolved database identifier before building the qualified probe SQL. As written, databases containing a quote make the probe fail and the outer check treats that failure as a pass, so the destination can still fail later during table creation.</comment>

<file context>
@@ -593,11 +595,87 @@ def precheck(self) -> None:
+
+    def _probe_table_creation(self, cursor: "TeradataCursor", *, database: str) -> bool:
+        """CREATE then DROP a throwaway table in ``database``. True iff CREATE got 3524."""
+        qualified = f'"{database}"."{_PRECHECK_PROBE_TABLE_PREFIX}{uuid4().hex[:16]}"'
+        try:
+            cursor.execute(f"CREATE TABLE {qualified} (probe_col INTEGER)")
</file context>

Suggested change

	qualified = f'"{database}"."{_PRECHECK_PROBE_TABLE_PREFIX}{uuid4().hex[:16]}"'
	escaped_database = database.replace('"', '""')
	qualified = f'"{escaped_database}"."{_PRECHECK_PROBE_TABLE_PREFIX}{uuid4().hex[:16]}"'

[tabossert]

Approving. Nothing here blocks. Everything except a 3524 fails open, unit CI is green (the SQL integration jobs were skipped, so no Teradata code ran beyond mocks), and the body is upfront that none of this has hit a live Teradata. Notes below, most important first; 1 and 2 are the ones I'd look at before merge.

1. A no-table destination that works today can now be refused, and the Impact line says it can't. Impact says "A destination that works today keeps passing: only 3524 refuses." Risk says a no-table user whose per-workflow table exists but who has since lost CREATE TABLE is now refused. Risk is the accurate one: teradata.py:619-623 runs the CREATE on every precheck when table_name is unset, and revoking CREATE after the first run is a normal least-privilege move. It's #811's no-wider rule this bends (sql.py:380-382, "Wider refuses a credential that works"), not the fail-open part. How much it matters depends on how often the platform runs precheck: the comment at :781-783 says orchestrators skip init(), so the probe does fire there. Is that once per job, or only on connector save/test? If per job, a workflow breaks the day a DBA tightens grants. Warning instead of refusing when table_name is unset would switch the refusal off on the path this PR mostly targets, so that's a trade-off rather than a fix. At minimum, correct the Impact line and add this case to the CHANGELOG, which currently only says a configured existing table is unchanged.

2. The probe asks for less than the real CREATE, and only 3524 can refuse it. The probe is CREATE TABLE "<db>"."unstructured_precheck_<hex>" (probe_col INTEGER) (:653-655). The real one in assets/teradata_elements_schema.sql is CREATE MULTISET TABLE with CLOB, VECTOR32, JSON and a PRIMARY INDEX. #811's rule is that the probe's surface matches the real statement, "no wider, no narrower" (sql.py:380-381). If VECTOR32 is a SYSUDTLIB type that needs UDTUSAGE (I couldn't confirm that from the docs), a user with CREATE TABLE but no UDTUSAGE passes precheck and fails at upload. Every missing-UDTUSAGE case I found comes back as 3523, so moving the probe to the real DDL only closes the gap if it also refuses on 3523, with a message that doesn't claim the missing right is CREATE TABLE. I get why it's 3524-only today: the message names CREATE TABLE, and 3524 is the code that's certainly about that (the test comment at test_teradata.py:2276 and the Risk section say as much). One option: run the asset unqualified under the probe name, the way create_destination() runs it in the session database (:575-579). That matches the real surface exactly and also sidesteps cubic's quoting point, since the database name never gets interpolated.

3. With Database blank, the refusal points at a fix the customer probably doesn't want. In row B the message says to grant CREATE TABLE on admin_db, but the user only lands in admin_db because it's the session default (database comes from SELECT DATABASE at :617, raised at :630-634, "Grant ... on that database" from sql.py:626). A DBA who does exactly what it says gets the outcome the customer complained about: tables auto-created in a database nobody chose. When connection_config.database is unset, something like "or set the Database field to the database the table should go in" would help.

4. Question on row C. The client follow-up is a run that succeeded and put its table in a database nobody picked. That's row C, which per the body still passes and still lands there; the new signal is the INFO line in precheck (:618). Is that tracked somewhere else? Cheap improvement while you're here: create_destination() already has current_db (:566), so :576 could log "creating table X in database Y", which puts the database on the line that actually creates the table and not only in precheck.

5. The outer except in check_create_table_permission can drop a confirmed refusal. denied is set inside with self.get_cursor() (:615-623). If cursor.close() (:351) or get_connection()'s finally: commit() / close() (:338-342) raises after the CREATE got 3524, the except at :624 logs "inconclusive" and returns. I don't know whether teradatasql's commit() can actually raise there under autocommit, and #811's _run_write_probe has the same shape, so this is hardening, not a regression. It's cheap though: initialise denied = False before the try and don't return from the except once it's True.

6. Probe tables leaked by a killed process can't be traced. A failed DROP, including one on a dropped connection, logs the table name (:672-676), which is good. The one unlogged path is the process dying between CREATE and DROP: a pod kill, or the preflight timeout if it kills the check rather than just stopping waiting (I couldn't tell which from the body). Each run gets a fresh uuid4 name and nothing sweeps them. The window is two statements, so low priority. If you do add a sweep, bound it by CreatorName = USER and an age well past any precheck's runtime, or it can drop a concurrent precheck's in-flight probe under the same credential.

7. Test gaps.

• Nothing reaches the outer except at :624-628: no test makes SELECT DATABASE raise or return None, makes the DBC.TablesV lookup or get_cursor() raise, or makes teardown raise (see 5). A non-driver CREATE error hits the inner except at :656 instead, and that's untested too.
• The integration test calls precheck() on a table that doesn't exist yet (test/integration/connectors/sql/test_teradata.py:359), so the probe runs live there, but nothing checks that no unstructured_precheck_% table is left behind. That's a cheap assertion to add. When the restricted-user run from the Proof section happens, it'd be worth landing as a test like postgres's test_postgres_destination_precheck_refuses_a_credential_that_cannot_write (test_postgres.py:247). That's what confirms 3524 is what the server actually sends.

8. Version vs #816. This is 1.11.18 and #816 is 1.11.17, both off 1.11.16. Both change line 1 of __version__.py and the top of the CHANGELOG, so whichever merges second conflicts. If that resolution keeps 1.11.17 the version goes backwards, and scripts/version-sync.sh only rejects a version equal to main's (:125), so CI won't catch it. Skipping .17 suggests #816 is meant to go first, but both bodies say merge order: none.

9. The body's wire-contract text doesn't match the code. The body quotes upload-path 3524 as "Teradata error 3524 (user does not have CREATE TABLE access to the database)". The descriptor at :121 is "user does not have the required access to the database", with for '<table>' appended (:198-205), and test_teradata.py:1329 pins the code's wording.

10. 3524 is reclassified on the source side too. _USER_FAULT_TERADATA_CODES is shared by both directions, so the indexer's SELECT TOP 1 probe (direction="source", :372-395) now raises UserError rather than SourceConnectionError on 3524. Probably rare on a SELECT, but the CHANGELOG and Impact only mention the upload path.

11. Nits.

• The classify_write_denial docstring still says "Only the three codes" (:709); it's four now.
• Logs can't tell "probe passed" from "skipped, table exists": both only emit the :618 line (early return at :622, success at :677). One INFO line per outcome would help triage. Inconclusive is also warning here (:625, :666) vs info in _run_write_probe (sql.py:554).
• _table_exists filters TableKind = 'T' (:646, same as main), so an existing NoPI table ('O', I believe) counts as absent. That job already fails today at create_destination(); the only change is that precheck now calls it a CREATE TABLE problem.

Q. Was EXPLAIN CREATE TABLE <real DDL> considered? I couldn't verify from the docs whether Teradata checks access rights on EXPLAIN of DDL, but if it does you'd get the same answer with no dictionary write and no leftover risk.

cubic's quoting note on :653 is already in the thread, so I didn't repeat it.

[tabossert]

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

[paulkarayan]

Thanks for this. Answering all of it, but the ordering matters: this PR merged before the follow-up work landed, so nothing below is a commit on this branch. It is on pk/teradata-precheck-review-followup off main, as 1.11.19, and it needs its own PR. The two things I could still do here I did: the body is corrected (items 1 and 9), and this comment answers the rest.

1. Fixed, and you were right about which line was lying. The Impact line is corrected in the body. The Risk section was the accurate one: with no table configured, teradata.py:619-623 runs the CREATE on every precheck, so a user whose per-workflow table exists and who has since lost CREATE TABLE is refused even though create_destination() would have found the table and returned early without creating one. Precheck cannot know the per-workflow name, so it cannot tell the two cases apart. The body now says that, and says it bends #811's no-wider rule on that path.

Your gating question, answered from the orchestration code rather than guessed. Observed, three separate triggers:

• Per job, when enable_preflight is on: platform-etl-orchestration/.../plugins_controller/main.py:237-246 runs run_preflight_gate as a hard barrier before /invoke, one /precheck per node per run. The flag is off on several customer environments, and a precheck timeout is treated as PASS by that controller.
• Every UI "test connection": platform-api/.../routers/nodes.py:139-175 publishes CheckNodeConnectionV1, and a one-shot check_executioner calls /precheck. Not job-coupled.
• Connector save through the public API: no. There is a comment at platform-api/.../public_api/dependencies.py:66 saying the real fix is to do plugin prechecks there, so it is not implemented today.

So yes, per job, wherever preflight is enabled. A workflow does break the day a DBA tightens grants.

I kept the refusal rather than downgrading to a warning, and I think that is the right call, but it is your trade-off and I want to be explicit about it. Warning when table_name is unset switches the check off on exactly the path the platform uses, which is the path this PR exists for. Instead the refusal now names the escape hatch that already existed in the code and was not being mentioned: set the Table Name field, and the lookup in check_create_table_permission skips the CREATE probe entirely and falls through to the 1.11.16 INSERT/DELETE probe. That turns a dead end into something the customer can act on, without removing the check. New tests cover the hint firing when the field is unset and staying off when it is set. The case is in the CHANGELOG under 1.11.19 rather than edited into the released 1.11.18 entry.

2. Fixed, and your unqualified suggestion is what it does. The probe now runs create_destination()'s own statement through the shared _elements_schema_sql() under the throwaway name, so it asks for the real DDL with CLOB, VECTOR32, JSON and the PRIMARY INDEX, not a one-INTEGER-column stand-in. It runs unqualified, the way create_destination() runs it, so it lands in the session database the check just resolved and no identifier reaches the SQL text, which also closes cubic's quoting point. 3523 now refuses alongside 3524, and its message deliberately does not tell the customer CREATE TABLE is the missing right, because on that code it is not.

3. Fixed. Both refusal messages now carry a hint when connection_config.database is blank: the database named is only the session default, and the Database field exists. A DBA following the message otherwise grants rights on a database nobody chose.

4. Answered, and the cheap improvement is in. create_destination() now logs "creating table X in database Y" on the line that actually creates the table, using the current_db it already had, so the database is no longer only visible in precheck. I do not know of row C being tracked anywhere else; if it is, point me at it and I will link them.

5. Fixed. denied is initialised before the try, and the outer except no longer returns once it is set: a cursor close or the connection's own commit raising after the server refused the CREATE leaves the refusal standing instead of turning it into "inconclusive". Tested, including teardown raising after a confirmed refusal.

6. Partly fixed, sweep declined. The traceability gap is closed: the probe table is named in a log line before the CREATE runs, so a process killed between CREATE and DROP leaves a record of the name. I did not add a sweep. Your own warning is the reason: bounded by CreatorName = USER and an age it is still a precheck issuing DROPs against customer objects, and getting the bound wrong drops a concurrent precheck's in-flight probe under the same credential. That is a worse failure than the leak, and the window is two statements.

7. Fixed. All the paths you listed have tests now: SELECT DATABASE raising and returning nothing, the DBC.TablesV lookup raising, get_cursor() raising, teardown raising after a refusal, and a non-driver CREATE error hitting the inner except. The integration test asserts leftover_probe_tables() == [] after the precheck that really runs the probe, with _ escaped in the LIKE so the prefix does not match by accident. The restricted-user test modelled on test_postgres_destination_precheck_refuses_a_credential_that_cannot_write is not written: it needs a live Teradata and a second credential, and I have neither. That leg is waived, not done. What unblocks it is a reachable Vantage with a user I can revoke CREATE TABLE from.

8. Confirmed, and it has already happened. main is 1.11.18 and #816's head is still 1.11.17, and #816 is now CONFLICTING. I checked version-sync.sh: the guard only fires when the updated version equals main's, so a resolution that keeps 1.11.17 goes backwards and CI passes. #816 needs a bump to 1.11.19 or later when it is rebased, and my follow-up branch takes 1.11.19 today, so one of us has to move. I have not touched #816.

9. Fixed in the body. The real message is Teradata error 3524 (user does not have the required access to the database) for '<table>'. The body said "user does not have CREATE TABLE access to the database", which is not what _USER_FAULT_TERADATA_CODES holds and not what test_teradata.py:1329 pins. Corrected, with the source-side note from item 10 attached to the same bullet.

10. Fixed in the CHANGELOG and the body. _USER_FAULT_TERADATA_CODES is consulted at every classification site, so TeradataIndexer.precheck's SELECT TOP 1 probe raises UserError rather than SourceConnectionError on 3524. That is now written down in both places instead of only the upload path. On the audience question it is the right classification in both directions: 3524 is the customer's grant to fix whichever way the statement runs, and the audience is a property of the raise site, not of the direction.

11.

• Fixed: the docstring says "the four codes".
• Fixed: skipped, refused, created and inconclusive each get their own line, and inconclusive is info to match _run_write_probe.
• Declined, TableKind = 'T'. _table_exists is shared with create_destination(), so "absent" means the same thing to the check and to the upload by construction. Widening it in precheck alone would pass a case create_destination() still fails, which is the narrower-probe failure fix(sql): refuse a destination credential that cannot write #811 forbids. Widening both is a real change to the auto-create path and does not belong in a review follow-up. Worth its own issue if you want the NoPI case handled.

Q. Yes, and it is documented, but I did not swap to it. Teradata's SQL Data Manipulation Language manual (B035-1146, 17.00, EXPLAIN Request Modifier, "Required Privileges") says: "To EXPLAIN a request, you must have the permissions that are required to execute that request." The mechanism holds up: SQL Request and Transaction Processing (B035-1142) puts Security between the Resolver and Query Rewrite, upstream of the Optimizer that produces the EXPLAIN output, so an EXPLAIN cannot reach the plan without passing the rights check. DDL is explainable and the manual carries a worked EXPLAIN CREATE TABLE. Note STATIC is the default and DYNAMIC EXPLAIN partially executes, so a bare EXPLAIN is the safe form.

Three reasons I left it as a candidate rather than doing it. The error code EXPLAIN returns on a refusal is inferred, not observed: nothing I found documents the number, and building the refusal on a code we have not seen come back from an EXPLAIN is the same mistake as the one-column probe. In the worked plan the "table already exists" check is a runtime ABORT TEST step from DBC.TVM, so EXPLAIN most likely does not surface 3803. Perm space is not evaluated either, so 2644 passes. It is a real improvement and it removes the dictionary write and the leftover entirely, so I would take it, but it wants one live run to confirm the code first. Same blocker as the item 7 test.

Nothing here has touched a live Teradata. That has not changed.